-
-
Notifications
You must be signed in to change notification settings - Fork 185
feat: support allowClear #728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@BadMooncc is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough为 InputNumber 增加 allowClear 清空功能:新增组件属性与回调、清空按钮 UI 与样式、示例、文档与测试,点击清空时通过 allowClear.clearValue 或 undefined 更新值并触发 onClear。(≤50 字) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as InputNumber UI
participant Comp as InputNumber Component
participant Props as Props Handler
User->>UI: 点击清空按钮
activate UI
UI->>Comp: onClear 按钮点击事件
activate Comp
alt 有 onClear 回调
Comp->>Props: 调用 onClear()
Note right of Props `#D3F9D8`: 可选回调触发
end
Comp->>Comp: 读取 allowClear.clearValue 或 使用 undefined
Comp->>Comp: 调用 triggerValueUpdate(clearValue)
Comp->>UI: 更新显示值(空或指定值)
deactivate Comp
deactivate UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @BadMooncc, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
感谢你的贡献!这个 PR 为 InputNumber 组件增加了 allowClear 功能,这是一个很棒的增强。整体实现很好,也包含了相应的文档和测试。
我发现了一个关于清除按钮可见性逻辑的小问题,它在值为 0 或在非受控模式下表现不正确。此外,测试用例也缺少了对这些场景的覆盖。
我在代码中留下了一些具体的修改建议,希望能帮助你完善这个功能。
| className={clsx(`${prefixCls}-clear-icon`, { | ||
| [`${prefixCls}-clear-icon-hidden`]: !(!disabled && !readOnly && value), | ||
| [`${prefixCls}-clear-icon-has-suffix`]: !!suffix, | ||
| })} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
清除按钮的可见性逻辑存在问题。当前使用 value prop 来判断,但这会导致在以下两种情况下按钮被错误地隐藏:
- 当
InputNumber的值为0时(因为!0的结果是true)。 - 当组件为非受控模式时(使用
defaultValue),valueprop 为undefined。
建议改用内部状态 decimalValue 来进行判断,它能更准确地反映组件的当前值是否为空。这样可以确保在值为 0 或使用 defaultValue 时,清除按钮也能正确显示。
| className={clsx(`${prefixCls}-clear-icon`, { | |
| [`${prefixCls}-clear-icon-hidden`]: !(!disabled && !readOnly && value), | |
| [`${prefixCls}-clear-icon-has-suffix`]: !!suffix, | |
| })} | |
| className={clsx(`${prefixCls}-clear-icon`, { | |
| [`${prefixCls}-clear-icon-hidden`]: disabled || readOnly || decimalValue.isEmpty(), | |
| [`${prefixCls}-clear-icon-has-suffix`]: !!suffix, | |
| })} |
| const clearIcon = container.querySelector('.rc-input-number-clear-icon'); | ||
| expect(clearIcon).toBeTruthy(); | ||
| expect(clearIcon).not.toHaveClass('rc-input-number-clear-icon-hidden'); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议在此处增加一个测试用例,用于验证当 value 为 0 时清除图标的可见性。这是一个重要的边界情况,因为 0 是一个假值(falsy value),可能会导致可见性判断出错。
你可以添加如下测试:
it('should render clear icon when value is 0', () => {
const { container } = render(<InputNumber allowClear value={0} />);
const clearIcon = container.querySelector('.rc-input-number-clear-icon');
expect(clearIcon).not.toHaveClass('rc-input-number-clear-icon-hidden');
});| ); | ||
|
|
||
| const clearIcon = container.querySelector('.rc-input-number-clear-icon'); | ||
| fireEvent.click(clearIcon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
assets/index.less (1)
97-105: 建议添加鼠标样式以改善用户体验清除按钮的基础样式正确。为了提升用户体验,建议添加
cursor: pointer以及悬停状态的视觉反馈。可选的改进:
&-clear-icon { padding: 0; font-size: 12px; background: none; border: none; + cursor: pointer; + &:hover { + opacity: 0.7; + } }tests/allowClear.test.tsx (1)
6-14: 建议增强可见性验证测试验证了清除图标元素的存在,但没有明确验证它是否可见(即没有
rc-input-number-clear-icon-hidden类)。建议添加显式的可见性检查:
it('should render clear icon when allowClear is true and value is not empty', () => { const { container } = render( <InputNumber allowClear value={123} /> ); const clearIcon = container.querySelector('.rc-input-number-clear-icon'); expect(clearIcon).toBeTruthy(); expect(clearIcon).not.toHaveClass('rc-input-number-clear-icon-hidden'); + // Verify icon is visible + expect(clearIcon).toBeVisible(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
assets/index.less(1 hunks)docs/api.md(2 hunks)docs/demo/allow-clear.tsx(1 hunks)docs/example.md(1 hunks)src/InputNumber.tsx(5 hunks)tests/allowClear.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-09-29T07:54:50.892Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: tests/validator.test.tsx:34-83
Timestamp: 2024-09-29T07:54:50.892Z
Learning: In `tests/validator.test.tsx`, the test cases focus on the `validator` function. When the validator condition is not met, the input value should not change. The `formatter` is tested separately in another file.
Applied to files:
tests/allowClear.test.tsx
📚 Learning: 2024-09-29T06:18:11.993Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:173-174
Timestamp: 2024-09-29T06:18:11.993Z
Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
Applied to files:
tests/allowClear.test.tsxdocs/demo/allow-clear.tsxsrc/InputNumber.tsxdocs/api.md
📚 Learning: 2024-10-08T21:56:37.546Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:393-395
Timestamp: 2024-10-08T21:56:37.546Z
Learning: The `InputNumber` component does not use error states or messages; it is designed to prevent the value from updating if validation fails without displaying additional feedback to the user.
Applied to files:
docs/demo/allow-clear.tsxsrc/InputNumber.tsx
🧬 Code graph analysis (1)
src/InputNumber.tsx (1)
docs/demo/allow-clear.tsx (1)
value(6-25)
🔇 Additional comments (4)
src/InputNumber.tsx (1)
101-101: 新增的属性类型定义合理
allowClear和onClear属性的类型定义清晰且符合预期。支持布尔值和对象形式的allowClear提供了良好的灵活性。Also applies to: 120-120
docs/example.md (1)
16-18: 文档更新正确新增的 allow-clear 示例章节格式正确,与其他示例保持一致。
docs/demo/allow-clear.tsx (1)
6-25: 示例代码清晰有效演示组件正确展示了
allowClear功能的使用方式,包括自定义clearValue以及与prefix/suffix的配合使用。docs/api.md (1)
79-84: API 文档完整准确新增的
allowClear和onClear属性文档清晰明了,类型定义和描述都准确无误。Also applies to: 157-162
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
assets/index.less (1)
97-105: 改进清除按钮的交互样式当前样式缺少交互反馈,建议添加:
- 鼠标指针样式
- 悬停和焦点状态
应用以下改进:
&-clear-icon { padding: 0; font-size: 12px; background: none; border: none; + cursor: pointer; + color: rgba(0, 0, 0, 0.45); + transition: color 0.3s; + + &:hover { + color: rgba(0, 0, 0, 0.85); + } + + &:focus { + outline: none; + color: #1890ff; + } } &-clear-icon-hidden { display: none; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
assets/index.less(1 hunks)docs/api.md(2 hunks)docs/demo/allow-clear.tsx(1 hunks)docs/example.md(1 hunks)src/InputNumber.tsx(5 hunks)tests/allowClear.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/demo/allow-clear.tsx
- tests/allowClear.test.tsx
- docs/example.md
- docs/api.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-29T06:18:11.993Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:173-174
Timestamp: 2024-09-29T06:18:11.993Z
Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
Applied to files:
src/InputNumber.tsx
📚 Learning: 2024-10-08T21:56:37.546Z
Learnt from: bombillazo
Repo: react-component/input-number PR: 644
File: src/InputNumber.tsx:393-395
Timestamp: 2024-10-08T21:56:37.546Z
Learning: The `InputNumber` component does not use error states or messages; it is designed to prevent the value from updating if validation fails without displaying additional feedback to the user.
Applied to files:
src/InputNumber.tsx
🔇 Additional comments (2)
src/InputNumber.tsx (2)
101-101: 属性类型定义合理
allowClear支持布尔值或对象配置(带clearValue),类型定义清晰且灵活。
120-120: 回调定义正确
onClear回调的类型定义符合预期。
English Template / 英文模板
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
📝 更新日志
Summary by CodeRabbit
发布说明
新功能
样式
文档
测试